fix Notice: Undefined index: 0777 in Stat->mergeMeta#19
Conversation
…ergeMeta() (line 157 of \vendor\twistor\flysystem-stream-wrapper\src\Flysystem\Plugin\Stat.php).
ms-slasher13
changed the title
|
The appveyor failed on chocolatey windows package manager. This looks like it should fix |
|
Hey, |
|
@twistor I too would love to see this or a similar fix merged and released, as it should fix PHP 7.3/7.4 compatibility. |
|
+1 |
|
@twistor ping |
benjifisher
left a comment
benjifisher
left a comment
There was a problem hiding this comment.
Thanks for this improvement! This change fixes the problem for my current project.
I made some suggestions: one little tweak to the code and several changes to be consistent with the commenting standards used in this project.
| /** | ||
| * Normalize a permissions string. | ||
| * | ||
| * @param string $permissions |
There was a problem hiding this comment.
| * @param string $permissions | |
| * @param string $permissions A permissions string, such as '644' or | |
| * 'drw-rw-r--' |
| * | ||
| * @param string $permissions | ||
| * | ||
| * @return int |
There was a problem hiding this comment.
| * @return int | |
| * @return int The numeric version of the permissions. |
| return $permissions & 0777; | ||
| } | ||
|
|
||
| // remove the type identifier |
There was a problem hiding this comment.
| // remove the type identifier | |
| // Remove the type identifier. |
| // remove the type identifier | ||
| $permissions = substr($permissions, 1); | ||
|
|
||
| // map the string rights to the numeric counterparts |
There was a problem hiding this comment.
| // map the string rights to the numeric counterparts | |
| // Map the string rights to the numeric counterparts. |
| $map = ['-' => '0', 'r' => '4', 'w' => '2', 'x' => '1']; | ||
| $permissions = strtr($permissions, $map); | ||
|
|
||
| // split up the permission groups |
There was a problem hiding this comment.
| // split up the permission groups | |
| // Split up the permission groups. |
| // split up the permission groups | ||
| $parts = str_split($permissions, 3); | ||
|
|
||
| // convert the groups |
There was a problem hiding this comment.
| // convert the groups | |
| // Convert the groups. |
| return array_sum(str_split($part)); | ||
| }; | ||
|
|
||
| // converts to decimal number |
There was a problem hiding this comment.
| // converts to decimal number | |
| // Converts to decimal number. |
| } | ||
|
|
||
| // remove the type identifier | ||
| $permissions = substr($permissions, 1); |
There was a problem hiding this comment.
| $permissions = substr($permissions, 1); | |
| $permissions = substr($permissions, -9); |
This will work with or without the initial d or - for regular file vs. directory. I think this also makes it clear that we expect 9 characters.
I considered padding with - characters to ensure 9 characters, but I am not sure when that would be likely to make a difference. Pad on the left or the right? Maybe it is a bad idea to fix problems that no one has.
There was a problem hiding this comment.
I feel like the original code is closer to what the comment says it's trying to do, but indeed -9 is clearer for the actual intent here: extracting the permission groups in isolation.
Padding would make it mildly safer to use with the code below it, and if you're using substr from the right then I'd say it makes sense to pad the left, if at all.
| $permissions = substr($permissions, 1); | |
| $permissions = substr(str_pad($permissions, 9, '-', STR_PAD_LEFT), -9); |
The comment could be updated to:
// Extract the permission groups.The trickiest thing is how vague the file metadata is with Flysystem, but I suppose it has to be (for v1.x at least 😉).
7 participants
fix undefined index in mergeMeta
Notice: Undefined index: 0777 in Twistor\Flysystem\Plugin\Stat->mergeMeta() (line 157 of \vendor\twistor\flysystem-stream-wrapper\src\Flysystem\Plugin\Stat.php)using flysystem 1.0.63 and "Local" adapter
see #12
see #15